Harden the Scala parser against a real-world corpus - #8519
Draft
knutwannheden wants to merge 55 commits into
Draft
Harden the Scala parser against a real-world corpus#8519knutwannheden wants to merge 55 commits into
knutwannheden wants to merge 55 commits into
Conversation
Sweeping 1,693 real Scala files (cats-effect plus scala3's own library and compiler) through the parser surfaced 103 files that fail the print-idempotency check, which makes each of them a ParseError and blocks ingestion. This fixes the eight defects behind roughly a third of them, each with a minimal reproducer added to the matching existing test class. Most share a root cause: the parser advances a manual cursor, but also jumps it to a dotty span end, and anything the cursor never consumed in between is silently dropped or smuggled into a Space. - Comma-separated parents. `class C extends A, B` printed as `extends Awith, B` because `with` was hard-coded as the separator, so the `,` was never consumed and leaked into the next parent's prefix. Scala 3 accepts either, and the two may be mixed, so each parent now records the separator introducing it. - Primary-constructor access modifier. `final class X private ()` lost the whole clause, because a constructor was only recognized when the next non-whitespace character was `(`. - Clause keyword ordering. `(using @deprecated ctx: String)` printed as `( @deprecatedusing ctx: String)`; `using`/`implicit` open the clause and now print ahead of parameter annotations, on both the constructor and method paths. - Higher-kinded variance. `F[-_, +_]` printed as `F[_, _]`. - Literal types. `def f(): true` printed as `true.type`. Adds S.LiteralType, mirroring JS.LiteralType, holding an Expression so `-1` is covered too. - Type-parameter variance modeling. `+`/`-` was crammed into the identifier's name, so `Foo[+A]` had a type parameter literally named `+A`. It is a modifier and now lives in J.TypeParameter.modifiers, as Kotlin already does. - Scala 3 end markers. `end X` was dropped or left sitting inside a Space, producing an invalid LST. Now captured on class, object, trait, def, val, given, if, while and match, braced and braceless. A `val`'s marker sits beyond dotty's span for the definition, so it is claimed by name, which is also the language's rule and stops a nested definition taking its parent's marker. The corpus goes from 103 to at most 72 failing files. Remaining families are capture-checking syntax, parenthesized types used as parents, unmapped `new`, and polymorphic function types.
Probing what non-whitespace can sit between a construct's own content and the end of dotty's span for it found comments and semicolons being swept into a Space. These produce an LST whose whitespace holds a comment instead of a Comment, so a recipe cannot see it and a formatting recipe could delete it. This class of defect is invisible to the parser's print-idempotency check, which only compares text, so these files parse successfully today and carry a silently invalid tree. - The compilation unit's EOF space was built with Space.build, which does no comment parsing, so any comment trailing the last statement landed in the whitespace field. It now goes through ScalaSpace.format like every other space in the parser. This covers a comment after an indented class body, method body, val, if or match. - An object body collected its statements with a bare JRightPadded, capturing no trailing separator, unlike the class body path which already used consumeTrailingSemicolon. Semicolons in class bodies, method bodies and blocks already round-tripped; only object bodies did not. - Compilation-unit statements were unpadded, so a `;` after a top-level class, import or val had nowhere to live. They are now JRightPadded and carry the Semicolon marker, and the printer walks the padding rather than the unwrapped statement list. Also removes a stray System.out.println from the converter.
ScalaSpace.format treated every `*` that was not preceded by `/` as comment text, so a wildcard import reaching a Space lost its selector: `import core.*` printed as `import core.`. Only a `*` inside a block comment belongs in the comment buffer; elsewhere it is ordinary prefix text. This surfaced once the compilation unit's EOF space started parsing comments. Files with chained package clauses put their whole body in that space, because a non-braced nested package is not modeled, so the wildcard passed through the formatter.
A non-braced nested package clause was skipped, so for
package dotty.tools
package dotc
class X
only the first clause became the package declaration and everything after it —
imports, classes, the whole body — landed in the compilation unit's EOF space.
The text round-tripped, so the print check stayed green while nothing in the
file was reachable to a recipe. The idiom is pervasive in Scala codebases that
mirror a deep directory layout.
A chained clause scopes everything after it exactly as a braced one scopes its
body, so it now becomes an S.PackageDeclaration whose block omits the braces,
reusing the shape braced packages already use.
Measured over 1,693 files of cats-effect and the scala3 library and compiler,
counting a file as sound only when it parses and its LST holds no source in
whitespace: 674 sound files before, 1,047 after. Parse errors rise from 74 to
205 because these bodies are now genuinely visited for the first time and meet
defects that were previously hidden behind the whitespace, chiefly unmapped
parenthesized function types and capture-checking syntax.
Both sit in a template between syntax the parser already handles, and neither had
anywhere to live, so their source was absorbed into a Space: the LST round-tripped
as text while hiding the clause from recipes.
A self-type (`trait T { self => ... }`, `this: U =>`) is kept out of the body by
dotty, so nothing claims it. It is captured between the body delimiter and the
first statement, where a `=>` can only be a self type because any body statement
starts later, and rides on the body block for class, trait and object bodies in
both braced and indented form.
A `derives` clause sits between the parent list and the body delimiter, so it is
split off the text that would otherwise become the body's prefix.
Over 1,693 files of cats-effect and the scala3 library and compiler, files that
parse and hold no source in whitespace go from 1,047 to 1,172.
`new (Int => Unit) { ... }` and `class C extends T with (Int => Unit)` put a
parenthesized function type in a parent position. Dotty parses the parentheses
as untpd.Parens, which the visitor mapped in expression position to
J.Parentheses — not a TypeTree — so a parent list either threw "Unmapped Scala
AST node: Parens" or failed casting to TypeTree, taking the whole file with it.
A parenthesized parent is a type, so it is now visited in type position and kept
parenthesized as a J.ParenthesizedTypeTree. Applied to the three parent sites:
a sole parent, a mixin in `new X with ...`, and a parent in a class declaration.
`transparent`, `inline`, `opaque` and `infix` were missing from the table of definition-level modifier keywords, so their source was swept into a Space and hidden from recipes. A primary constructor's access modifier was only recognized when a parameter list followed it, so `final abstract class Byte private extends AnyVal` lost the `private` the same way. The modifier is now claimed whether or not a parameter list follows, and the printer emits it in both cases. `open` is left out: dotty does not surface it the same way and adding it duplicated the declaration on print.
Scala 3 capture checking writes a capture set as a suffix, `IterableOnce[A]^` or
`C^{it}`. Dotty desugars it to a synthetic `retains` annotation that has no `@`
in source, so converting the annotation to a J.Annotation threw and took the
whole file with it: this was the largest single family of parse failures.
The suffix is now recognized where it sits and kept verbatim on the type it
follows, leaving annotated types and annotated expressions on their existing
paths. Over 1,693 files of cats-effect and the scala3 library and compiler,
parse errors drop from 202 to 167.
The `^{...}` form parses but still hides the `=` that follows a return type
carrying one, so those files remain unsound. That is an improvement on failing
to parse at all, and the bare `^` form round-trips.
Scala 2 procedure syntax is detected by scanning for whether a `{` or an `=`
comes first after the method name. A capture set writes a brace in the return
type, so `def f(): List[Int]^{this} = Nil` looked like `def f() { ... }` and the
`=` was never consumed, leaving it inside the body's prefix.
The scan already skips parameter and type-argument brackets; it now skips a
capture set the same way.
`f(using ctx)` puts the keyword between the `(` and the first argument, where nothing claimed it, so it was absorbed into that argument's prefix and hidden from recipes. It was the largest single cause of unsound trees. The keyword rides on the first argument rather than on the argument container, because every printer path reaches an argument through `visit`, while the container is opened by half a dozen different call sites. One override of `beforeSyntax` then emits it wherever the arguments are printed. Covers a plain call, a call on a select, a curried call's later list, several arguments, and the keyword on its own line. Over 1,693 files of cats-effect and the scala3 library and compiler, files that parse and hold no source in whitespace go from 1,222 to 1,300.
…itions The definition-level modifier table already knew these keywords, but a val, var or given runs through its own hand-rolled modifier scan, which stopped at the Scala 2 set. Anything it did not recognize stayed in the whitespace ahead of the keyword, so `inline val X = 8` hid the modifier from recipes. The scan now handles the same three keywords, following the shape of the branches beside it. `inline` on a parameter is still absorbed; that position is read through a different path and the untyped tree carries no flag for it.
An extension block and a try expression each ended by absorbing a trailing `end extension` or `end try` into a Space, the same way a class body did before end markers were modeled. Both now claim the marker before the cursor moves past it, reusing the existing mechanism. `end extension` is the most common end marker in the corpus by a wide margin. Two shapes remain absorbed: `end for` and `end new` after an indented body, where the body block's own end space claims the text first. A braced `for` with `end for`, and an indented `for` without one, both round-trip today.
… annotations Two print failures, both from reading source out of token order. A lambda took the first `=>` in its own source as its arrow, but a parameter's type can hold one first, so `(cb: Int => Unit) => 1` split at the type's arrow and re-emitted the tail of the parameter list. The search now starts after the parameter list. An annotation built its name without asking whether the source backticked it, so `@`inline`` printed as `@inline`. The identifier helper already takes a quoting flag; the annotation path now passes it. Over 1,693 files of cats-effect and the scala3 library and compiler, parse errors drop from 164 to 140.
`(A => Unit) => B` split at the arrow inside its own parenthesized parameter list, re-emitting the tail of that list and printing `=>>`. The arrow search now starts past a leading parenthesized group, matching the fix made for lambdas. Parse errors over the corpus drop from 140 to 131.
`def map[B](f: Int => B)(implicit @implicitNotFound(msg) ev: Ordering[B])` put the annotation before the keyword, giving `( @implicitNotFound(msg)implicit ev`. A curried parameter list prints through the lambda-parameter path, which had not been given the ordering already applied to constructor and method parameters. Parse errors over the corpus drop from 131 to 125.
Two diagnostic harnesses were committed by accident. They hard-code paths under /tmp and are not part of the module's test suite.
…tion arrow Two more places that read a name's length or an arrow's spelling from the tree rather than from the source in front of the cursor. A nested higher-kinded parameter took its bracket offset from dotty's name for the wildcard, `_$1`, which is three characters where the source has one, so `F[_[_], _]` re-emitted the inner kind list as `F[_[_[_], _], _]`. Capture checking writes a pure function as `A -> B`. With no `=>` to find, the whole type was taken as the parameter and a second `=> B` was appended. The arrow is now recognized and carried on a marker, beside the existing one for the context-function arrow `?=>`.
With capture checking enabled, dotty wraps a by-name parameter's result type in a `CapturesAndResult` node. It has no syntax of its own, so a plain `elem: => T` threw "Unmapped Scala AST node: CapturesAndResult" and failed the whole file. The node is now unwrapped to the type it carries. This was the largest remaining group of parse failures. Over the corpus they drop from 114 to 94.
Dotty's Apply span for `new M[A](x)(f)(g)` starts at the type, so the `new` keyword sits in the gap ahead of the span. The outer call absorbed it as whitespace and the NewClass then printed its own, giving `new newM[A]`. An application's prefix now stops at a `new` keyword and leaves the cursor on it, so the constructor call consumes it as it already does for two argument lists.
A companion object's ModuleDef span starts back at the companion class, so the guard that compared the span start against the cursor skipped the modifier scan entirely. The modifier then leaked into whitespace and the kind keyword was emitted twice: `private object B` printed as `privateobject object B`. The snippet was already taken from the cursor, which is past everything consumed so far, so only the guard needed to change.
The class path already split a trailing `end` marker out of a braceless body's end space; the object path did not, so `end Conversion` closing an indented object stayed in whitespace and the tree hid it from recipes. Objects are a common home for the marker, including at column zero after an extension block and on a nested object. Over 1,693 files of cats-effect and the scala3 library and compiler, files that parse and hold no source in whitespace go from 1,383 to 1,438, and end markers no longer appear among the causes of unsound trees.
Dotty records a context bound on the parameter's rhs only for a plain parameter. For a higher-kinded one the rhs is the kind's LambdaTypeTree, so the `: Monad` of `[F[_]: Monad]` was left unclaimed and absorbed into whitespace, hiding it from recipes. It is now read from source and printed after the name. This was the largest remaining cause of unsound trees.
Same shape as the context bound fixed alongside it: dotty gives a higher-kinded parameter's rhs as the kind itself, so `[It[a] <: Iterable[a]]` left the bound unclaimed and it was absorbed into whitespace. The capture now covers `:`, `<:` and `>:`, and the marker is named for bounds generally rather than context bounds alone.
Scala 3 allows a trailing comma before the closing paren of an argument list. The run before that paren was taken as whitespace, so the comma was hidden from recipes. It now rides on the last argument's padding through the TrailingComma marker the parameter lists already use, which the printer honours. Covers a call's arguments and a constructor's, and the shared builder used for annotations.
The untyped tree carries no flag for an inline parameter, and the parameter's span opens on the keyword rather than on the name, so neither the flag nor a search of the gap ahead of the name could find it and `inline op: Boolean` lost the modifier into whitespace. It is read from the source at the cursor, beside the existing handling for `using` and `implicit`. This was the largest remaining cause of unsound trees.
They hard-code paths under /tmp and are diagnostic harnesses, not part of the module's test suite. A directory-wide 'git add' re-added them after an earlier removal.
`(Context ?=> Symbol) @unchecked` and `Iterable[(K, V) @unchecked]` annotate a type written with parentheses. Visiting the annotated element in expression position gave a J.Parentheses or a tuple expression, neither of which is a TypeTree, so the annotation threw and took the file with it. Both forms are now visited as types, which also keeps them usable as expressions. Parse errors over the corpus drop from 81 to what these ten files free up.
Scala 3 allows a trailing comma before the brace closing an import's selector list. The loop consumed that comma as an ordinary separator and then found whitespace where it required the `}`, failing the whole file. The last selector now carries the comma on the TrailingComma marker the printer already honours.
…s `@` The printer decided between `val` and `var` by looking for a Final modifier, so an explicit `final` turned `final var` into `final val`. The two are orthogonal in Scala — `final` governs overriding, `val`/`var` mutability — and the standard library writes `private final var`. The keyword the source used is now recorded by the parser instead of being inferred. An annotation built its name without the run between the `@` and the name, so `String @ unchecked` printed as `String @unchecked`.
…arameters as parameters
Where the source wrote `def desc(sym: Int)= {`, the parser recorded no prefix for
the `=` and the printer fell back to the body's prefix, printing that space on
both sides of the `=`. The run is now recorded whenever an `=` is consumed, empty
or not.
An extension's parameters went through the general definition path, which reads a
name from the ValDef's span. That span opens on a modifier, so `extension (inline
x: String)` printed as `(inlinexnline x: String)`. They now use the parameter
visitor, which reads the modifier before the name.
A block comment ahead of `match`, as in `tp/*.dealias*/ match`, contains a `.` that made the parser read the expression as a selector-style `tp.match`, splitting the comment across the keyword space and the marker. The dot search now skips comment text, so only a real `.` selects that form.
The modifier scan matched any whole-word keyword in its text window, so a commented-out modifier such as the `/*final*/` of `private /*final*/ case class C` became a real modifier and split the comment across two spaces. Keyword lookup now uses the comment-aware search, which also covers the `val`/`var`/`given`, `class`/`trait`/`enum`, `object`, `case` and `package` lookups.
The loop that visits a method's curried parameter lists sat inside the branch for a non-empty first list, so `def f()(c: Int): Int` left its second list unconsumed. The return-type scan then took that list's `:` for the return-type colon and swallowed `Int):` into the return type's prefix, hiding it from recipes.
A curried method with a braceless single-statement body printed the statement without the block's own prefix. An auxiliary constructor keeps the space after the `=` there, because Dotty wraps its body in a block, so `def this()(implicit o: Ordering[K]) = this(null)` came back as `=this(null)`.
A type carrying a capture set took the space ahead of it from the source whenever its
extracted prefix was empty. Inside an enclosing type — the `|` of `Iterator[Int]^{this} | Null`
or the `=>` of a function type — that space belongs to the enclosing type, which had
already emitted it, so it was printed twice.
Scala 3 accepts `if (cond) then ...` as well as the parenless `if cond then ...`. Only the latter consumed the keyword, so in the former it rode along inside the then-branch's prefix, where recipes cannot see it. It is now kept on a marker and printed between the condition and the branch.
`for case (k, v) <- pairs` filters by pattern. Dotty leaves the keyword out of both the enumerator and the pattern it introduces, so it rode along in a Space where recipes cannot see it. It is now marked on the enumerator and printed ahead of the pattern.
Scala 3 accepts `while (cond) do ...` and `for (x <- xs) do ...` as well as the parenless forms. Only the latter consumed the keyword, so in the former it rode along inside the body's prefix, where recipes cannot see it. It is now kept on a marker and printed between the head and the body, for both the single-generator and multi-generator shapes of `for`.
Dotty models `new Foo { ... }` and `new Foo() { ... }` with the same empty `Apply`, and
the parser read the absent parentheses from that rather than from the source, so the
`()` of the second form ended up in the body's prefix. The list is now taken from the
source, where an empty container prints `()` and a null one prints nothing.
The scan for the modifiers of a `val`/`var`/`given` matched `private` and `protected` only ahead of the loop that reads the rest, so `implicit protected val x` kept the `protected` in the whitespace before the keyword, where recipes cannot see it. Access modifiers are now read by that loop like any other, scope suffix included.
A block's statement loop advanced the cursor to the next statement, past the comma that separates `import a.*, b.*`. The continuation reads back to that comma for its own prefix, so it found the cursor already beyond it and the file failed to parse. The loop now stops at the comma when the next statement is an import or export.
An `inline` parameter is read from the source, since the untyped tree carries no flag for it, but the lookup was skipped for a context parameter. In `def f(using inline x: T)` the keyword follows `using`, so it stayed in the whitespace between the two. The lookup now also runs once the `using` or `implicit` keyword has been consumed.
The keyword opens the if expression's span, and the parser advanced straight to the `if`, dropping it from the printed output and failing the file. It is now kept on a marker and printed ahead of the `if`, for both the parenless and parenthesized forms.
The scan for the keyword skipped only spaces and tabs, so a parameter list that puts each parameter on its own line kept the `inline` of `inline x: Int` as raw text in the whitespace ahead of it. It now skips whitespace of any kind. Also drops the arm for a Scala 2 `implicit` parameter, which the preceding condition already covers.
The search for the `()` of `new Foo() { ... }` ran to the end of the constructor call, so
it could reach a `(` belonging to a member of the body. It now looks only at the token
directly after the type, which is where the list opens if there is one.
The pair returned when no body delimiter is found carried a literal NUL character, which makes the file read as binary to grep and other text tools.
The package name was given a one-space prefix, so `package com.example` came back with the run collapsed to a single space. It is now read from the source, which also covers a name on the line below the keyword.
A catch clause whose handler is a partial-function expression rather than a list of `case` clauses has no cases to build from, and the clause was dropped along with the handler's source. The expression is now the body of the catch block.
Dotty's span for the object reaches past the closing brace to a trailing `end` marker, and
the body consumed the whole span, so `object O { ... }\nend O` lost the marker. The body
now stops at the brace and the marker is claimed by name, as it already is for a class.
The scan for `class X private (i: Int)` ran forward from the class name over any amount of whitespace, so a class with no constructor parentheses claimed the `private` of the next declaration: in `class Foo` followed by `private class Bar`, `Bar` came out with no modifiers. The text still round-tripped, because the verbatim marker reprinted it in the same place, which is why no test caught it. The scan now stops at the class's own span.
An argument list with no arguments left its parentheses unread, so `f(\n)` printed as `f()` and a comment between them was lost. The run is now carried by a J.Empty element, which the container prints between the parentheses.
The split around a trailing comma used the shared space parser, which ends a block comment at the first `*/`. Scala block comments nest, so `f(1, 2 /* x /* y */ z */,)` came back with the comment's tail mangled. Both halves now go through the Scala parser, as the no-comma branch already did.
`asInstanceOf` is a method on `scala.Any`, and Dotty gives `x.asInstanceOf[T]` the same tree as any other generic call, which the parser peeled off by name into a J.TypeCast: the expression came second, a J.ControlParentheses stood in for the `[...]`, and the run before the `.` needed a marker of its own because the node had no slot for it. The run after the `.` had no slot at all, so `x.` followed by a newline and `asInstanceOf[T]` moved the dot onto the second line. As a J.MethodInvocation every part has a home — the select's padding, the name's prefix, the type-parameter container — and AsInstanceOfPrefix is gone. The printer keeps rendering a J.TypeCast the Scala way for recipes that build one.
Type ascription is modelled by S.TypeAscription, and nothing has attached this marker since. Its javadoc described marking a J.TypeCast for `expr: Type`, which is the mapping rewrite-scala/CLAUDE.md rules out.
Several sites repeated why Dotty's span reaches past a definition to its end marker, and why a parenthesized annotated argument is a type. Each rule now sits at the helper it governs, and the comments that only restated the line below them are gone.
knutwannheden
force-pushed
the
wonky-frog
branch
from
August 17, 2026 08:47
fa9c90a to
83bd646
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The Scala parser round-tripped its own test suite cleanly, but that suite was the only thing measuring it. Running the parser over a real corpus — cats-effect plus the Scala 3 library and compiler, 1,693 files — showed a very different picture, and revealed that print-idempotency alone was measuring a small fraction of the problem.
There are two distinct failure modes, and only the first is loud:
Spaceinstead of being modelled. Recipes cannot see or transform that code, and the parser's own checks stay silent. At the start of this work, 961 files were in this state.Both are now measured on every change, so a fix that trades one for the other is visible rather than invisible. Over the corpus:
Most fixes fall into a few families: syntax that Dotty discards after checking it (
endmarkers, self-types,derives), keywords the untyped tree does not record (then,do,case,inline,using), spans that reach past the construct they describe, and text scans that matched inside comments.Examples
Syntax that has no home in the
Jmodel is carried on a marker and printed back, rather than riding along inside whitespace. Each of these now parses into an LST that a recipe can inspect:asInstanceOfchanges shape. It is a method onscala.Any, and Dotty givesx.asInstanceOf[T]the same tree as any other generic call, so it is now aJ.MethodInvocationrather than aJ.TypeCastwhoseJ.ControlParenthesesstood in for the[...]:The second form used to move the dot onto the following line, because the old shape had nowhere to put that whitespace — the same gap that had already produced the
AsInstanceOfPrefixmarker for the run before the dot. Both are gone.Summary
endmarkers, self-types,derives, capture sets,then/do/case/inlinekeywords, pure function arrows, kind-parameter variance, higher-kinded bounds, trailing commas, chained package clauses.asInstanceOfas the method invocation it is, retiring theAsInstanceOfPrefixmarker and the unusedTypeAscriptionmarker.matchselector dot, keyword lookups generally.package, the interior of an empty argument list, the body prefix of a curried method.Test plan
./gradlew :rewrite-scala:testgreen — 1,031 tests, up from 906.private class Barkeeps its modifier, and thatasInstanceOfparses to aJ.MethodInvocationwithIntas its single type argument.